Create constructor for creating allocated BlockArrays #39

dlfivefifty · 2017-11-26T12:20:55Z

• Support BlockArray{Float64}(1:3, 1:3), etc. syntax for creating allocated block arrays

• Deprecate BlockArray(blocks,...) in favour of _BlockArray(blocks,...) to avoid confusion with BlockArray(arr,...) function that converts arr to a BlockArray.

…ocated block arrays • Deprecate BlockArray(blocks,...) in favour of _BlockArray(blocks,...) to avoid confusion with BlockArray(arr,...) function that converts arr to a BlockArray.

dlfivefifty · 2017-11-26T12:39:02Z

This addresses #37

KristofferC · 2017-11-26T12:44:04Z

We should definitely not export a function with an underscore in it (or is this not exported anymore). Also, why can't we just dispatch on Type vs instance to keep it as one function?

Being able to create uninitialized blocks is valuable in my opinion.

dlfivefifty · 2017-11-26T12:47:37Z

It isn't exported.

The issue is that one might want to create a BlockArray{Matrix{Float64},2,Matrix{Matrix{Float64}}}. Dispatching differently when the first argument is a Matrix{Matrix{Float64}} vs a Matrix{Float64} is inconsistent. I also think BlockArray and PseudoBlockArray should have consistent syntax.

Because I've added allocating constructors, the need for the BlockArray(blocks,...) constructor is not as big, and I think since it's using internal data it's reasonable for users to need to use BlockArrays._BlockArray(blocks,...) if they want an unitialized BlockArray.

dlfivefifty · 2017-11-26T12:52:24Z

Alternatively, it could be renamed unallocated_BlockArray.

KristofferC · 2017-11-26T13:04:33Z

Base is doing something like BlockArray{...}(uninitialized, ...) wouldn't the same work here?

dlfivefifty · 2017-11-26T13:05:17Z

Cool! how would I implement that?

codecov-io · 2017-11-26T13:28:11Z

Codecov Report

Merging #39 into master will increase coverage by 1.18%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #39      +/-   ##
==========================================
+ Coverage    88.4%   89.58%   +1.18%     
==========================================
  Files           9        9              
  Lines         345      365      +20     
==========================================
+ Hits          305      327      +22     
+ Misses         40       38       -2

Impacted Files	Coverage Δ
src/blockarray.jl	`100% <100%> (+3.12%)`	⬆️
src/blocksizes.jl	`85.18% <100%> (+0.87%)`	⬆️
src/pseudo_blockarray.jl	`91.37% <100%> (+0.63%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d59bb0e...6cdf3ff. Read the comment docs.

fredrikekre · 2017-11-26T15:30:23Z

Cool! how would I implement that?

JuliaLang/Compat.jl#417

KristofferC · 2017-11-27T09:11:00Z

docs/src/man/blockarrays.md

+## Creating initialized `BlockArrays`
+
+A block array can be created with initialized blocks using the `BlockArray{T}(block_sizes)`
+function. The block_sizes are each an `AbstractVector{Int}` which determines the size of the blocks in that dimension. We here create a `[1,2]×[3,2]` block matrix of `Float32`s:


Perhaps we should mention that the arrays themselves are initialized, the values in them are not? What do you think?

Is this behaviour going to change in 0.7 so that Matrix{Float64}(n,m) automatically sets the values?

I don't think it will, but not sure.

I'm a bit confused about the difference between Matrix{Float64}(uninitialized, n, m) and Matrix{Float64}(n, m).

I'm worried that there's a confusion between uninitialized blocks and uninitialized entries with using uninitialized to mean uninitialized blocks. Especially since I'm hoping to making PseudoBlockArray and BlockArray have equivalent behaviour for the same syntax.

It might be better to add a UninitializedBlocks and then we have the following three cases:

BlockArray{Float64}(uninitialized_blocks, ns), which does not initialize the blocks, i.e., A.blocks[1] is #undef. This is the current behaviour of BlockArray(Vector{Float64}, ns)

BlockArray{Float64}(uninitialized, ns), which initializes the blocks, but not the entries of the blocks, i.e., A.blocks[1] is allocated as a Vector{Float64}(uninitialized, ns[1])

BlockArray{Float64}(ns), which does initialize the blocks, and the entries, i.e., A.blocks[1] is allocated as a Vector{Float64}(ns[1])

PseudoBlockArray{Float64}(uninitialized, ns), A. blocks is allocated as a Vector{Float64}(uninitialized, sum(ns))

PseudoBlockArray{Float64}(ns), which does initialize the blocks, and the entries, i.e., A.blocks is allocated as a Vector{Float64}(sum(ns))

KristofferC · 2017-11-27T09:11:29Z

docs/src/man/blockarrays.md

@@ -23,13 +38,13 @@ julia> BlockArray(Matrix{Float32}, [1,2], [3,2])
 We can also use a `SparseVector` or any other user defined array type:

 ```jl
-julia> BlockArray(SparseVector{Float64, Int}, [1,2])
+julia> BlockArrays._BlockArray(SparseVector{Float64, Int}, [1,2])


Perhaps this should be updated to use uninitialized?

dlfivefifty · 2017-12-02T09:16:03Z

@KristofferC The checks now pass, are you happy for this to be merged?

fredrikekre · 2017-12-02T10:10:10Z

src/pseudo_blockarray.jl

@@ -60,6 +60,32 @@ PseudoBlockArray(blocks::R, block_sizes::Vararg{AbstractVector{Int}, N}) where {
    PseudoBlockArray(blocks, Vector{Int}.(block_sizes)...)


+
+@inline function PseudoBlockArray{T}(block_sizes::BlockSizes{N}) where {T, N}


Presumably all these new PseudoBlockArray constructors should use uninitialized?

Yes I think you are right, even if the package supports 0.6.

My only hesitation is that sometimes changes in Julia master are backtracked on, but in this case that’s unlikely to happen.

fredrikekre · 2017-12-02T10:10:27Z

REQUIRE

@@ -1,2 +1,2 @@
 julia 0.6
-Compat 0.30.0
+Compat 0.38.0


Compat 0.39

It doesn't depend on anything in Compat 0.39 that's not in Compat 0.38, so I think this should be left.

Compat 0.38 is broken. Please update to 0.39.

I wrote this and then I merged 20 seconds later. Oh well, will update on master.

• Support BlockArray{Float64}(1:3, 1:3), etc. syntax for creating all…

9e1187c

…ocated block arrays • Deprecate BlockArray(blocks,...) in favour of _BlockArray(blocks,...) to avoid confusion with BlockArray(arr,...) function that converts arr to a BlockArray.

add unitialized_BlockArray, and fix doc strings

a3302fe

uninitialized_BlockArray(...) -> BlockArray(uninitialized, ...)

f12e957

KristofferC reviewed Nov 27, 2017

View reviewed changes

dlfivefifty and others added 3 commits November 27, 2017 19:50

increase coverage, updated man

42d7a49

REQUIRE Compat 0.38

9e5ad98

0.7 tests pass

526d846

fredrikekre reviewed Dec 2, 2017

View reviewed changes

Always uses uninitialized, adds unitialized_blocks for BlockArray

6cdf3ff

KristofferC merged commit d88a881 into master Dec 2, 2017

KristofferC deleted the dl/fillarrays branch December 2, 2017 14:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create constructor for creating allocated BlockArrays #39

Create constructor for creating allocated BlockArrays #39

dlfivefifty commented Nov 26, 2017

dlfivefifty commented Nov 26, 2017

KristofferC commented Nov 26, 2017 •

edited

Loading

dlfivefifty commented Nov 26, 2017 •

edited

Loading

dlfivefifty commented Nov 26, 2017

KristofferC commented Nov 26, 2017

dlfivefifty commented Nov 26, 2017

codecov-io commented Nov 26, 2017 •

edited

Loading

fredrikekre commented Nov 26, 2017

KristofferC Nov 27, 2017

dlfivefifty Nov 27, 2017

KristofferC Nov 27, 2017

dlfivefifty Nov 28, 2017 •

edited

Loading

KristofferC Nov 27, 2017

dlfivefifty commented Dec 2, 2017

fredrikekre Dec 2, 2017

dlfivefifty Dec 2, 2017

fredrikekre Dec 2, 2017 •

edited

Loading

dlfivefifty Dec 2, 2017

KristofferC Dec 2, 2017

KristofferC Dec 2, 2017

		@@ -60,6 +60,32 @@ PseudoBlockArray(blocks::R, block_sizes::Vararg{AbstractVector{Int}, N}) where {
		PseudoBlockArray(blocks, Vector{Int}.(block_sizes)...)



		@inline function PseudoBlockArray{T}(block_sizes::BlockSizes{N}) where {T, N}

Create constructor for creating allocated BlockArrays #39

Create constructor for creating allocated BlockArrays #39

Conversation

dlfivefifty commented Nov 26, 2017

dlfivefifty commented Nov 26, 2017

KristofferC commented Nov 26, 2017 • edited Loading

dlfivefifty commented Nov 26, 2017 • edited Loading

dlfivefifty commented Nov 26, 2017

KristofferC commented Nov 26, 2017

dlfivefifty commented Nov 26, 2017

codecov-io commented Nov 26, 2017 • edited Loading

Codecov Report

fredrikekre commented Nov 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlfivefifty Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlfivefifty commented Dec 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredrikekre Dec 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KristofferC commented Nov 26, 2017 •

edited

Loading

dlfivefifty commented Nov 26, 2017 •

edited

Loading

codecov-io commented Nov 26, 2017 •

edited

Loading

dlfivefifty Nov 28, 2017 •

edited

Loading

fredrikekre Dec 2, 2017 •

edited

Loading